Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added Generated Files For SPIR-V Builder #841

Open
wants to merge 21 commits into
base: master
Choose a base branch
from

Conversation

Yey007
Copy link
Contributor

@Yey007 Yey007 commented Sep 3, 2022

Hello!

Sorry for the delay on this PR, but I think I'm finally done. There are, however, a few minor issues I want to ask about:

  1. Naming conventions: because this is generated code, they're hard to follow. I can do my best, but I don't know if it's worth the added complexity in the generator.
  2. There are a few performance concerns that we may want to address in the future depending on whether this is going to be fast enough for our needs. For example, in SPIRVWord.cs there is an extra allocation here that could be avoided by using Marshal
- return new SPIRVWord(BitConverter.ToUInt32(bytes.ToArray(), 0));
+ return new SPRIVWord(MemoryMarshal.Read<int>(bytes));

but there is a readability tradeoff. There are other minor things like this that can be improved if necessary.
3. Finally, the signature of Merge for builders currently looks like this:

void Merge(ISPIRVBuilder other);

which is not type safe, as the compiler will allow a string builder to be merged with a binary builder. Can anyone think of a way of making this type safe without having to resort to ISPRIVBuilder generic over the implementing type, which would require specifying the builder we want to use everywhere and throw away the advantages of using an interface?

Thanks for reviewing!

@m4rs-mt m4rs-mt added this to the v2.X milestone Sep 4, 2022
@m4rs-mt m4rs-mt added the feature A new feature (or feature request) label Sep 4, 2022
@Yey007
Copy link
Contributor Author

Yey007 commented Sep 4, 2023

Oops, looks like some of the Site files got added to this commit somehow. I'll fix that.

@Yey007
Copy link
Contributor Author

Yey007 commented Sep 4, 2023

Question: What should I do about the Rider files that got committed? I'm guessing we don't want those.

@Hong-Xiang
Copy link

Hi, thanks for this awesome project, I'm really looking forward to use SPIR-V backend. I'm trying to build a C# to WGSL (WebGPU's shader language, which should be convertible with SPIR-V) and other shading languages transpiler, after some experiment using ILSpy and syntax level translation, I found it might be better to define a IR for target language is really required (instead of directly translating C# AST into target language string. Although I what I want to build is more graphics/rendering focused, but a runtime compiler from C# IL to SPIRV might be enough for this.

I'm trying to find out if there is possibility to contribute on this, but currently struggling in understanding how to add SPIRV backend. Could you please tell me is there any suggestions on how to start learn/plan or steps could be done to add SPIRV backend? Thanks

@Yey007
Copy link
Contributor Author

Yey007 commented Sep 20, 2024

Hi! Sorry for the late reply.

To add SPIR-V as a backend, you would probably need to implement a Backend using the SPIR-V builder that is introduced in this PR. The existing OpenCL and PTX backends may be good to look at for inspiration---they both may have relevant pieces. I imagine the SPIR-V backend might end up being more similar to the PTX backend just because it's also a bytecode, but I'm not sure which language SPIR-V has more in common with. I would also check out the SPIR-V specification.

The other piece that would probably be needed is a Vulkan runtime. Again, the CUDA and OpenCL runtimes are probably a good reference, though I don't know much about Vulkan at all, so I can't say how similar the process would be.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature A new feature (or feature request)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants